Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Draft][CPU][Ref] Support Reduce ops with empty input #27603

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

xuchen-intel
Copy link
Contributor

@xuchen-intel xuchen-intel commented Nov 19, 2024

Details:

  1. [x64] Avoid the divisor in reduce_kernel_post_process to be zero, and enable post ops fusion of ReduceMean.
  2. [x64] Add axesZeroDim and axesZeroDimFusing in test cases, so that all these new added test cases will go exactly to the new added "early return" code block, where input tensor is empty and output tensor is not.
  3. [arm] makeExecutor is skipped for the case of empty input on ARM, because Acl library does not support empty input tensor (e.g., NEReduceMean::validate return error). Besides, because of early return, the executor won't be needed anyway.
  4. [arm] ARM Transformations ConvertReduceProd(Min, Max, Sum) are disabled to avoid empty output.

Tickets:

@xuchen-intel xuchen-intel added category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin labels Nov 19, 2024
@xuchen-intel xuchen-intel requested review from a team as code owners November 19, 2024 07:18
@xuchen-intel xuchen-intel requested review from itikhono and removed request for a team November 19, 2024 07:18
@github-actions github-actions bot added category: transformations OpenVINO Runtime library - Transformations category: TEMPLATE OpenVINO Template plugin labels Nov 19, 2024
Copy link
Contributor

@mitruska mitruska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks promising, thanks for support!

Comment on lines 21 to 22
{{{}, {{0, 19, 2, 2, 9}}}},
{{{}, {{1, 0, 0, 2, 9}}}},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are failing with
Supported Reduce executor is not found or primType is unexpected : undef_f32 Expected : acl_f32
It's worth to verify with master, because some cases could be originally unsupported as well.

Comment on lines 2280 to 2313
if (dstMemPtr->getSize() > 0) {
init_dst_data(dst_data, dstMemPtr->getSize());
reduce_kernel_post_process(dst_data);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the previously added check to skip post process if attr.get()->post_ops_.len() == 0 redundant?

testing::Values(ElementType::undefined),
testing::ValuesIn(inputShapes_Dynmic_ZeroDim)),
testing::Values(emptyCPUSpec),
testing::ValuesIn(fusingParamsSet_KeepNoDims),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the tests for other fusions have been enabled, great!
Have you found what was the root cause of the failures for fusions other than Swish?

@xuchen-intel xuchen-intel force-pushed the fix/reduce_empty branch 2 times, most recently from 5133b7e to f9d53fc Compare November 25, 2024 06:01
@xuchen-intel xuchen-intel force-pushed the fix/reduce_empty branch 2 times, most recently from 362b15d to 5aacfd7 Compare November 25, 2024 06:36
@yuxu42 yuxu42 added this to the 2024.6 milestone Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin category: TEMPLATE OpenVINO Template plugin category: transformations OpenVINO Runtime library - Transformations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants